Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support ruby 3 and kwargs #1158

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

jdelStrother
Copy link

Based on the work @webhoernchen started in #1130.

We've been running this in production for a month while upgrading an installation from ruby 2.7 to 3.0 without issue. I think I've managed to keep support for ruby 2.6, but haven't done much more than run the test suite on it.

@unixmonkey
Copy link

@jdelStrother Could you fix the conflicts in this PR to make it mergable? This is working great for me on a Ruby 3.1 project, and I'd love to see it get released.

@jdelStrother
Copy link
Author

jdelStrother commented May 6, 2022

If the maintainers are interested in merging this I’ll fix up the conflicts, but I’m no longer using DJ. Feel free to take over this branch if you’re more invested

@c960657
Copy link

c960657 commented Feb 5, 2024

You should also pass add **kwargs to the method generated dynamically by handle_asynchronously:

define_method(with_method) do |*args|
curr_opts = opts.clone
curr_opts.each_key do |key|
next unless (val = curr_opts[key]).is_a?(Proc)
curr_opts[key] = if val.arity == 1
val.call(self)
else
val.call
end
end
delay(curr_opts).__send__(without_method, *args)
end

@c960657
Copy link

c960657 commented Aug 15, 2024

I use this in production:

    define_method(with_method) do |*args, **kwargs|
      curr_opts = opts.clone
      curr_opts.each_key do |key|
        next unless (val = curr_opts[key]).is_a?(Proc)
        curr_opts[key] = if val.arity == 1
          val.call(self)
        else
          val.call
        end
      end
      delay(curr_opts).__send__(without_method, *args, **kwargs)
    end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants